Build options validation#333
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #333 +/- ##
=======================================
Coverage 91.78% 91.78%
=======================================
Files 362 362
Lines 23735 23735
=======================================
Hits 21786 21786
Misses 1949 1949 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ddaspit
left a comment
There was a problem hiding this comment.
@ddaspit reviewed 9 files and all commit messages, and made 7 comments.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on Enkidu93 and pmachapman).
machine/jobs/nmt_build_options.py line 24 at r1 (raw file):
fp16: bool | None = None tf32: bool | None = None save_strategy: str | None = None
Remove save_strategy.
machine/jobs/nmt_build_options.py line 34 at r1 (raw file):
) device: int | None = None
Remove device.
machine/jobs/nmt_build_options.py line 35 at r1 (raw file):
device: int | None = None num_beams: int | None = 2
The default should be None.
machine/jobs/build_clearml_helper.py line 125 at r1 (raw file):
def update_settings(settings: Settings, args: dict, task: Optional[Task], logger: logging.Logger, model: type[T]):
The T typevar isn't necessary. model: BaseModel should be sufficient.
machine/jobs/build_clearml_helper.py line 136 at r1 (raw file):
raise TypeError(f"Build options could not be parsed: {e}") from e try: model.model_validate(build_options)
It would be good to add tests to check the validation.
machine/jobs/build_clearml_helper.py line 139 at r1 (raw file):
except ValidationError as e: raise ValueError(f"Invalid build options: {e}") from e settings.update({settings.model_type: build_options})
This is a preexisting bug, but the build options are not getting merged in correctly for thot models. The model_type of thot should map to thot_align for alignment models and thot_mt for MT models.
pyproject.toml line 63 at r1 (raw file):
charset-normalizer = "^2.1.1" urllib3 = "<2" pydantic = "^2.13.4"
pydantic should be included in the jobs extra.
Part of sillsdev/serval#966
Fixes sillsdev/serval#306
This change is